What the check actually tests

The new 404 branch in _send is intended to implement the MCP spec rule: "When a client receives HTTP 404 in response to a request containing an Mcp-Session-Id, it MUST start a new session." The inline comment above the branch restates this exactly — "a 404 in response to a request that carried an Mcp-Session-Id".

But the predicate this._sessionId !== undefined is evaluated after the response header handling that runs unconditionally at the top of _send's response processing:

const sessionId = response.headers.get('mcp-session-id');
if (sessionId) {
    this._sessionId = sessionId;          // mutation happens here, even on !response.ok
}

if (!response.ok) {
    // ...401/403 handling...
    if (response.status === 404 && this._sessionId !== undefined) {  // reads post-mutation state
        this._sessionId = undefined;
        throw new SdkError(SdkErrorCode.ClientHttpSessionExpired, ...);
    }
}

So the check answers "is a session ID set now, after the response has been processed?" — not "did the request we just sent carry one?". Those two questions diverge in two ways.

How they diverge

(1) A 404 response that itself carries an mcp-session-id header. A request goes out with no session ID; the server responds 404 and includes mcp-session-id in the response headers. Lines a few up store it into this._sessionId; the 404 check then sees a non-undefined session ID, misclassifies the error as ClientHttpSessionExpired, and immediately clears the session ID it just stored. Per the spec this should be ClientHttpNotImplemented (the request had no session). The PR's negative test (treats a 404 without a session ID as a generic HTTP error) only mocks headers: new Headers(), so this path is untested.

(2) Concurrency. Two _send calls in flight: request A is an initialize whose 200 response sets this._sessionId; request B was dispatched earlier with no session ID and gets a 404. If B's response is processed after A's, the 404 check reads A's freshly-established session ID, misclassifies B's 404, and clears the session A just created. The session is wedged until the caller reconnects.

Step-by-step proof (scenario 1)

1. A fresh transport (no sessionId option) calls send(initialize). _commonHeaders() returns headers without mcp-session-id because this._sessionId is undefined.
2. The (misbehaving) server responds 404 Not Found with header mcp-session-id: abc.
3. response.headers.get('mcp-session-id') → 'abc' → this._sessionId = 'abc'.
4. !response.ok → enter error block. The 404 check sees this._sessionId === 'abc' !== undefined → true.
5. Throws SdkError(ClientHttpSessionExpired) and clears this._sessionId. Per the spec this is wrong — the request had no Mcp-Session-Id, so it should be a generic ClientHttpNotImplemented.

Why this is low-severity

Both triggers are unlikely in practice: real servers don't set mcp-session-id on 404 error responses, and the Protocol layer serializes initialize so the concurrency window is narrow (it requires direct transport.send() use outside Client.connect()). I'm filing this as a nit — a spec-precision issue with a one-line fix, not a likely real-world failure.

Fix

Snapshot the request-side state before the fetch and branch on that:

const hadSessionId = this._sessionId !== undefined;   // before await fetch
// ...
if (response.status === 404 && hadSessionId) { ... }

Or equivalently, check headers.has('mcp-session-id') on the already-built request headers.

The mirrored hunk in _startOrAuthSse happens to read correctly today (the GET path never writes the response mcp-session-id header back into this._sessionId), but using the same snapshot pattern there keeps the two branches symmetric and future-proofs against someone adding session-header handling to the GET path.

What the bug is

This PR introduces the rule "a 404 in response to a request carrying Mcp-Session-Id means the session expired server-side" and applies it to two of the three code paths in StreamableHTTPClientTransport that can carry a session ID: _send (POST) and _startOrAuthSse (GET). The third path, terminateSession(), also always carries a session ID — it returns early at the top of the method when !this._sessionId — but is left untouched.

The code path that triggers it

async terminateSession(): Promise<void> {
    if (!this._sessionId) {
        return; // No session to terminate
    }
    // ... DELETE with Mcp-Session-Id header via _commonHeaders() ...
    if (!response.ok && response.status !== 405) {
        throw new SdkError(SdkErrorCode.ClientHttpFailedToTerminateSession, ...);
    }
    this._sessionId = undefined;   // never reached on 404
}

On a 404 the method falls into the !response.ok && status !== 405 branch and throws, so this._sessionId = undefined on the next line never executes.

Step-by-step proof

1. Client establishes a session; _sessionId = 'abc'.
2. Server-side, the session expires (idle timeout, restart, etc.).
3. Client calls transport.terminateSession().
4. The early-return guard passes (_sessionId is set), so a DELETE is sent with Mcp-Session-Id: abc.
5. Server replies 404 Not Found because it has no record of session abc.
6. response.ok is false, response.status !== 405, so ClientHttpFailedToTerminateSession is thrown.
7. this._sessionId is still 'abc' — the dead ID survives, and the caller is told termination failed even though the session is already gone (which is exactly the outcome the caller wanted).

Why this is the same wedge the PR is fixing

The PR description frames the motivation as "the session stays wedged until the app is restarted" because consumers couldn't distinguish session expiry from other HTTP failures. terminateSession() exhibits the identical symptom: after step 7 the transport still holds a stale session ID, and the surfaced error code (ClientHttpFailedToTerminateSession) is misleading — it implies an actionable failure when in fact nothing went wrong.

Why the impact is bounded (and why this is a nit rather than a blocker)

• terminateSession() is typically called on shutdown, so the lingering _sessionId is usually about to be discarded anyway.
• With this PR's other changes in place, if the caller does keep the transport alive and issues a subsequent POST, the new _send 404 handler will now clear the stale ID — so the wedge is no longer permanent the way it was before this PR.

That said, the inconsistency is real and the misleading error is user-visible: callers that surface or log termination failures will report a spurious error.

How to fix it

Treat 404 like the existing 405 special-case: when the DELETE returns 404, clear _sessionId and return successfully (the user's intent — terminating the session — has already been achieved server-side). Alternatively, throw ClientHttpSessionExpired after clearing _sessionId for consistency with the other two paths, but silent success is closer to the spec semantics for DELETE. Either way, _sessionId must be cleared.

// Option A: silent success (preferred — spec considers the session already terminated)
if (!response.ok && response.status !== 405 && response.status !== 404) {
    throw new SdkError(SdkErrorCode.ClientHttpFailedToTerminateSession, ...);
}
this._sessionId = undefined;

Test-coverage gap: GET SSE (_startOrAuthSse) 404-with-session-ID path is untested

This PR adds the "404 with a session ID means session expiry" handling to two code paths in packages/client/src/client/streamableHttp.ts:

1. _send (POST) — around line 649: throws SdkError(SdkErrorCode.ClientHttpSessionExpired, Session expired (HTTP 404): ${text}, { status: 404, text })
2. _startOrAuthSse (standalone GET SSE) — around line 291: throws SdkError(SdkErrorCode.ClientHttpSessionExpired, 'Failed to open SSE stream: session expired (HTTP 404)', { status: 404, statusText: response.statusText })

These are distinct branches with different error messages and different data shapes ({ status, text } vs { status, statusText }). The new describe('session expiry (HTTP 404)') block in packages/client/test/client/streamableHttp.test.ts only exercises path (1):

• The parametrized it.each(...) test calls sessionTransport.send(message), which routes through _send (POST).
• The negative case ("404 without a session ID") also calls transport.send(message) (POST).

Nothing in the test file ever invokes _startOrAuthSse (or transport.start() + a GET SSE attempt) with a session ID set and a mocked 404 response, so the GET-side branch is unverified.

Why this matters

The repo's review checklist requires that "new behavior has vitest coverage including error paths." Because the GET branch is a separate if with its own message and a divergent data shape, a future regression there (e.g. someone removing the GET-side branch during a refactor, or changing the data shape) would not be caught by the existing tests — they'd all stay green.

Concrete walk-through

1. Construct a StreamableHTTPClientTransport with { sessionId: 'existing-session-id' }.
2. Call await transport.start() then await (transport as any)._startOrAuthSse({}) (the test file already uses this private-method-access pattern for several other GET-path tests).
3. Mock fetch to resolve with { ok: false, status: 404, statusText: 'Not Found', text: () => Promise.resolve(''), headers: new Headers() }.
4. Currently no test asserts that the rejection is an SdkError with code === SdkErrorCode.ClientHttpSessionExpired, that error.data equals { status: 404, statusText: 'Not Found' }, or that transport.sessionId is cleared afterward.

Suggested fix

Add a GET-path test inside the existing describe('session expiry (HTTP 404)') group, something like:

it('treats 404 on the standalone GET SSE stream with a session ID as session expiry', async () => {
    const sessionTransport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
        sessionId: 'existing-session-id'
    });

    (globalThis.fetch as Mock).mockResolvedValueOnce({
        ok: false,
        status: 404,
        statusText: 'Not Found',
        text: () => Promise.resolve(''),
        headers: new Headers()
    });

    await sessionTransport.start();
    const error = await (sessionTransport as unknown as { _startOrAuthSse: (o: StartSSEOptions) => Promise<void> })
        ._startOrAuthSse({})
        .then(() => null, e => e);

    expect(error).toBeInstanceOf(SdkError);
    expect((error as SdkError).code).toBe(SdkErrorCode.ClientHttpSessionExpired);
    expect((error as SdkError).data).toEqual({ status: 404, statusText: 'Not Found' });
    expect(sessionTransport.sessionId).toBeUndefined();

    await sessionTransport.close().catch(() => {});
});

A matching negative case (GET 404 with no session ID still produces ClientHttpFailedToOpenStream) would round out the symmetry with the POST tests.

This is a test-coverage gap, not a runtime defect — the production GET-path code looks correct as written.